feat(rds): add support for database instances #2187

jogold · 2019-04-05T15:57:21Z

Add support for database instances.

Specific classes are exposed for a source database instance (DatabaseInstance), an instance created from a snapshot (DatabaseInstanceFromSnapshot) and a read replica instance (DatabaseInstanceReadReplica).

Add construct for option groups and refactor parameter groups.

Add basic support for instance event rules.

Integration with Secrets Manager and secret rotation.

Closes #2075
Closes #1693

Pull Request Checklist

Testing
- Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change)
- CLI change?: coordinate update of integration tests with team
- cdk-init template change?: coordinated update of integration tests with team
Docs
- jsdocs: All public APIs documented
- README: README and/or documentation topic updated
Title and Description
- Change type: title prefixed with fix, feat will appear in changelog
- Title: use lower-case and doesn't end with a period
- Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
- Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
Sensitive Modules (requires 2 PR approvers)
- IAM Policy Document (in @aws-cdk/aws-iam)
- EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
- Grant APIs (only if not based on official documentation with a reference)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

eladb

Just started the review of this huge piece of work. Looks great. More to review.

eladb · 2019-04-08T22:30:37Z

packages/@aws-cdk/aws-rds/README.md

@@ -45,33 +45,85 @@ By default, the master password will be generated and stored in AWS Secrets Mana
 Your cluster will be empty by default. To add a default database upon construction, specify the
 `defaultDatabaseName` attribute.

+### Starting an Instance Database
+To set up a instance database, create an instance of `DatabaseInstance`. You must


We use the terminology "define a DatabaseInstance" (instead of "create an instance").

eladb · 2019-04-08T22:34:30Z

packages/@aws-cdk/aws-rds/lib/instance.ts

+    rule.addEventPattern({
+      source: [ 'aws.rds' ],
+      resources: [
+        this.node.stack.formatArn({


Should we expose instanceArn?

eladb · 2019-04-08T22:35:27Z

packages/@aws-cdk/aws-rds/lib/instance.ts

+ */
+export interface DatabaseInstanceNewProps {
+  /**
+   * The name of the compute and memory capacity classes.


Is there a sensible default here?

eladb · 2019-04-08T22:35:58Z

packages/@aws-cdk/aws-rds/lib/instance.ts

+  /**
+   * Specifies if the database instance is a multiple Availability Zone deployment.
+   *
+   * @default false


What's the implications of making this true by default? Wouldn't that be a better practice?

Yes, if possible I would like to a have production defaults in this construct: db.r5.large, multi az, 1000 IOPS, 100 GiB, encryption enabled with default master key, 7 days backup, enhanced monitoring, no auto minor version upgrade (the console experience when selecting Production).

Looking at the discussion we had here #2063 (comment) and the issue you raised with implicit costs to customer, would you be ok to accept these production defaults?

@eladb any update on whether production defaults are acceptable for this construct?

That's an interesting conversation, and I wonder if this would be something we run into in the future, sort of "presets". The RDS console definitely demonstrates that there is value in allowing people to being configuring their new database from a predefined set of options that are tailored for a use case.

Maybe we should have a similar experience:

new rds.DatabaseInstance(this, 'instance', rds.DatabaseInstancePresets.production()); // or, override some value: new rds.DatabaseInstance(this, 'instance', rds.DatabaseInstancePresets.production({ multiAz: false }));

Something like that?

How would you specify mandatory properties like engine and masterUsername with this API? Spread the preset and add those properties?

{ ...rds.DatabaseInstancePresets.production(), masterUsername: '...' }

eladb · 2019-04-08T22:36:32Z

packages/@aws-cdk/aws-rds/lib/instance.ts

+   * The number of I/O operations per second (IOPS) that the database provisions.
+   * The value must be equal to or greater than 1000.
+   *
+   * @default no iops


What does no IOPs mean?

This is actually setting provisioned IOPS. The default behavior is you don't get provisioned (aka guaranteed) IOPS... This can incur additional cost when enabled.

So yeah, I guess the phrasing can be improve to this effect. I think copying the documentation from RDS/CloudFormation should be best.

eladb · 2019-04-08T22:36:41Z

packages/@aws-cdk/aws-rds/lib/instance.ts

+  /**
+   * The number of CPU cores and the number of threads per core.
+   *
+   * @default no processor features


What's the default?

eladb · 2019-04-08T22:38:20Z

packages/@aws-cdk/aws-rds/lib/instance.ts

+   *
+   * @default false
+   */
+  readonly enableIAMDatabaseAuthentication?: boolean;


Would it be sufficient to just call this iamAuth or something more concise?

I would go for iamAuthentication

Ideally, there should also be a grant method to provide permissions there. And this is tricky because it requires to know the instance's Resource ID, which I don't reckon is surfaced by CloudFormation (aka requires CustomResource 😔)

aka requires CustomResource

Another use case for #1850

eladb · 2019-04-08T22:38:54Z

packages/@aws-cdk/aws-rds/lib/instance.ts

+  /**
+   * The daily time range during which automated backups are performed.
+   *
+   * @default no preference


Does that mean random time range or no backup?

"no preference" means AWS chooses for you.

jogold · 2019-04-09T20:28:27Z

packages/@aws-cdk/aws-rds/lib/instance.ts

+  /**
+   * The storage type.
+   *
+   * @default GP2


This is not correct: the current implementation defaults to IOPS not GP2. Waiting for a reply on production defaults to update.

This still needs to be updated (wrong default value documented). @eladb any feedback on using production defaults here ? See #2187 (comment)

…low multi user

jogold · 2019-05-30T20:13:55Z

Added a commit to close #2075 and #1693

jogold added 7 commits April 2, 2019 16:07

Remove unused cluster-parameter-group-ref.ts

ef9a9cf

Refactor parameter group

1c17af4

Extract endpoint

e55b6fc

Add option group

8c31724

Make parameter group description optional

cc49f71

Add instance

d798e0c

Update README

6cc4e88

jogold requested a review from a team as a code owner April 5, 2019 15:57

jogold added 5 commits April 5, 2019 18:03

Merge branch 'master' into rds-instance

c890989

Fix import for option group

2e5b611

Use cdk.SecretValue

ee391c8

Add metrics

fe17f60

Add connections to option groups

2aed438

jogold mentioned this pull request Apr 8, 2019

feat(rds): Add InstanceParameterGroup #2075

Closed

4 tasks

eladb reviewed Apr 8, 2019

View reviewed changes

jogold added 6 commits April 9, 2019 12:33

README

9249aff

Merge branch 'master' into rds-instance

3b9bc6e

selectSubnets

4150e19

Expose instanceArn

8078dcf

Props and defaults

b91e3a3

JSDoc

737bcaa

jogold commented Apr 9, 2019

View reviewed changes

jogold added 8 commits April 10, 2019 18:17

Refactor engine and secret rotation to avoid mapping functions and al…

b817d92

…low multi user

README

6dd669d

JSDoc

a9b8792

Fix sem ver of rotation app

34627f8

JSDoc

305db7b

Improve integ test

ab5c6f3

Merge branch 'master' into rds-instance

96f14ee

use IResource and Resource

7c5526a

jogold added 2 commits May 30, 2019 21:39

Merge branch 'master' into rds-instance

6f8eec8

set GP2 as default storage type

d044e64

update README for events

c904e99

rix0rrr merged commit b864041 into aws:master May 31, 2019

jogold mentioned this pull request Jun 7, 2019

v0.34.0 #2791

Merged

4 tasks

jogold deleted the rds-instance branch June 13, 2019 13:20

This was referenced Jan 20, 2020

[Snyk] Upgrade @aws-cdk/aws-route53 from 0.22.0 to 0.39.0 MechanicalRock/tech-radar#20

Closed

[Snyk] Upgrade @aws-cdk/aws-s3 from 0.22.0 to 0.39.0 MechanicalRock/tech-radar#21

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(rds): add support for database instances #2187

feat(rds): add support for database instances #2187

jogold commented Apr 5, 2019 •

edited

Loading

eladb left a comment

eladb Apr 8, 2019

eladb Apr 8, 2019

jogold Apr 9, 2019

eladb Apr 8, 2019

eladb Apr 8, 2019

jogold Apr 9, 2019 •

edited

Loading

jogold Apr 24, 2019

eladb May 30, 2019

jogold May 30, 2019

eladb Apr 8, 2019

RomainMuller Apr 16, 2019

eladb Apr 8, 2019

eladb Apr 8, 2019

jogold Apr 9, 2019

RomainMuller Apr 16, 2019

jogold Apr 16, 2019

eladb Apr 8, 2019

RomainMuller Apr 16, 2019

jogold Apr 9, 2019 •

edited

Loading

jogold May 30, 2019

jogold commented May 30, 2019

feat(rds): add support for database instances #2187

feat(rds): add support for database instances #2187

Conversation

jogold commented Apr 5, 2019 • edited Loading

Pull Request Checklist

eladb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jogold Apr 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jogold Apr 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jogold commented May 30, 2019

jogold commented Apr 5, 2019 •

edited

Loading

jogold Apr 9, 2019 •

edited

Loading

jogold Apr 9, 2019 •

edited

Loading